feat: add flag and label completion for bazel commands #373

oncilla · 2023-01-15T11:35:17Z

This change adds auto completion for bazel flags on all the bazel sub-commands.

aspect build --sand<tab><tab>
--sandbox_add_mount_pair           --sandbox_explicit_pseudoterminal
--sandbox_base                     --sandbox_fake_hostname
--sandbox_block_path               --sandbox_fake_username
--sandbox_debug                    --sandbox_tmpfs_path
--sandbox_default_allow_network    --sandbox_writable_path

Flag completion for aspect specific flags is not affected:

aspect build --aspect:<tab><tab>
config       (User-specified Aspect CLI config file. /dev/null indic…)
interactive  (Interactive mode (e.g. prompts for user input)

This change also adds auto completion support for startup flags:

aspect --ho<tab><tab>
--home_rc           --host_jvm_args     --host_jvm_debug    --host_jvm_profile

Furthermore, this change adds label completion on the appropriate commands.
The logic is similar to the bazel logic:

https://github.com/bazelbuild/bazel/blob/d1bbf4b7698e019c11b7d1c483eb0b4959954060/scripts/bazel-complete-template.bash#L272-L278

However, not everything is behaving exactly the same, e.g., a custom
BAZEL_COMPLETION_PACKAGE_PATH is not supported yet.
For regular setups, this solution should work without any issues though.

fixes #355

CLAassistant · 2023-01-15T11:35:22Z

All committers have signed the CLA.

gregmagolan · 2023-01-24T02:58:07Z

Thanks for the PR!

I tried it and out read over the code and its a great start to getting this feature in. Performance is my main concern with this.

I measured ~2.5 seconds on my machine when I hit tab on this one before zsh showed the completion prompt:

% aspect build --<tab>
zsh: do you wish to see all 1266 possibilities (635 lines)?

and the number of options returned is too large to be helpful 🙄

Another test with only a single valid completion,

aspect build --memory_<tab>

=>

aspect build --memory_profile

also took ~2.5s.

We've been floating the idea around of vendoring in the bazel flags for each Bazel version so that we don't have to to ask Bazel itself what the flags are every time aspect runs. That should reduce the time to getting completion results.... although I haven't measured what ate up the ~2.5 seconds above. It might partially be a function of how many flags bazel has.

Also regarding performance, the behind the scenes aspect runs when you hit <tab> will presently start up any plugins that are configured which will add additional overhead when waiting for completion. Plugins can contribute commands & flags for those commands but for an MVP of this feature we can avoid auto-completion for plugin commands and configure the behind the scenes runs to ignore all plugin configuration.

Target completion comes with a bit overhead as well for reading and parsing files from disk. Perhaps some caching could be done here to speed things up. Bazel docs for zsh completion mention caching as an option:

# This way the completion script does not have to parse Bazel's options
# repeatedly.  The directory in cache-path must be created manually.
zstyle ':completion:*' use-cache on
zstyle ':completion:*' cache-path ~/.zsh/cache

To start I think we should measure why completion performance is poor and see what can be done to improve it. I think we should also separate flag completion & target completion into separate PRs with flags coming first. Making target completion fast seems like a harder problem that can be punted to a follow-up once we have flags working fast.

oncilla · 2023-01-25T19:20:39Z

I tried it and out read over the code and its a great start to getting this feature in. Performance is my main concern with this.
I measured ~2.5 seconds on my machine when I hit tab on this one before zsh showed the completion prompt:

That is orders of magnitudes slower than what I observe in bash:

$ time bazel run //:aspect -- __complete build -- >/dev/null
Loading: 
Loading: 0 packages loaded
Analyzing: target //:aspect (0 packages loaded, 0 targets configured)
INFO: Analyzed target //:aspect (0 packages loaded, 0 targets configured).
INFO: Found 1 target...
[0 / 1] [Prepa] BazelWorkspaceStatusAction stable-status.txt
Target //cmd/aspect:aspect up-to-date:
  bazel-bin/cmd/aspect/aspect_/aspect
INFO: Elapsed time: 0.154s, Critical Path: 0.02s
INFO: 1 process: 1 internal.
INFO: Build completed successfully, 1 total action
INFO: Running command line: bazel-bin/cmd/aspect/aspect_/aspect __complete build --
INFO: Build completed successfully, 1 total action
Completion ended with directive: ShellCompDirectiveDefault

real    0m0.416s
user    0m0.303s
sys     0m0.095s

Is this maybe an issue with zsh?
(Note that this is not from cold start, and bazel server is already running in the background)

and the number of options returned is too large to be helpful 🙄

While this is true, I think this is the least bad option. Not reporting flags is worse IMO.

We've been floating the idea around of vendoring in the bazel flags for each Bazel version so that we don't have to to ask Bazel itself what the flags are every time aspect runs. That should reduce the time to getting completion results.... although I haven't measured what ate up the ~2.5 seconds above. It might partially be a function of how many flags bazel has.

Do you want to vendor this in the repository? If so, any preferred location where to store it?
I think it would be pretty easy to do with go embeddings and a simple script to generate the files.

Target completion comes with a bit overhead as well for reading and parsing files from disk. Perhaps some caching could be done here to speed things up. Bazel docs for zsh completion mention caching as an option:

I don't really observe what you are seeing locally. Target completion is pretty fast for me.
Did you run this against some other repository with a lot of files?

$ time bazel run //:aspect -- __complete //pkg/aspect > /dev/null
Loading: 
Loading: 0 packages loaded
Analyzing: target //:aspect (0 packages loaded, 0 targets configured)
INFO: Analyzed target //:aspect (0 packages loaded, 0 targets configured).
INFO: Found 1 target...
[0 / 3] [Prepa] BazelWorkspaceStatusAction stable-status.txt
Target //cmd/aspect:aspect up-to-date:
  bazel-bin/cmd/aspect/aspect_/aspect
INFO: Elapsed time: 0.136s, Critical Path: 0.02s
INFO: 1 process: 1 internal.
INFO: Build completed successfully, 1 total action
INFO: Running command line: bazel-bin/cmd/aspect/aspect_/aspect __complete //pkg/aspect
INFO: Build completed successfully, 1 total action
Completion ended with directive: ShellCompDirectiveDefault

real    0m0.504s
user    0m0.401s
sys     0m0.107s

gregmagolan · 2023-01-25T19:25:43Z

Oh interesting that bash is that much faster zsh. I will check again with bash and report back.

oncilla · 2023-02-07T09:19:42Z

@gregmagolan any news here?

I really want to start using aspect CLI for its plugin system. But without completion, I will have a hard time introducing it at my day job.

gregmagolan · 2023-02-27T04:44:52Z

pkg/bazel/bazel_flags.go

+	// List of all commands with label as inputs. To compile the list, you can
+	// use the following command:
+	//
+	//   bazel help completion | grep 'ARGUMENT="label"'


Looks like there are label-test and label-bin variants so the correct grep is | grep 'ARGUMENT="label' (without the 2nd double quote) to catch the test, coverage and run commands.

I'll push a commit to update the list

$ bazel help completion | grep 'ARGUMENT="label' BAZEL_COMMAND_AQUERY_ARGUMENT="label" BAZEL_COMMAND_BUILD_ARGUMENT="label" BAZEL_COMMAND_COVERAGE_ARGUMENT="label-test" BAZEL_COMMAND_CQUERY_ARGUMENT="label" BAZEL_COMMAND_FETCH_ARGUMENT="label" BAZEL_COMMAND_MOBILE_INSTALL_ARGUMENT="label" BAZEL_COMMAND_PRINT_ACTION_ARGUMENT="label" BAZEL_COMMAND_QUERY_ARGUMENT="label" BAZEL_COMMAND_RUN_ARGUMENT="label-bin" BAZEL_COMMAND_TEST_ARGUMENT="label-test"

gregmagolan · 2023-02-27T04:56:52Z

pkg/bazel/bazel_flags.go

+	}
+	var flags []string
+	bazelFlags.VisitAll(func(f *pflag.Flag) {
+		flags = append(flags, "--"+f.Name)


hmm. suppose the --noable flags should technically be handled as well but that will nearly double the flag count 🤔

The --noable flags should be take care of:

bazel run //cmd/aspect -- __complete build "--" | grep '\-\-no' ... --notrim_test_configuration --nouse_action_cache --nouse_ijars --nouse_singlejar_apkbuilder --nouse_top_level_targets_for_symlinks --nouse_workers_with_dexbuilder --noverbose_explanations --noverbose_failures --nowatchfs --noworker_quit_after_build --noworker_sandboxing --noworker_verbose --nozip_undeclared_test_outputs

gregmagolan · 2023-02-27T05:14:00Z

Performance looks good with bash. Wonder what is up with zsh?

gregmagolan · 2023-02-27T05:33:06Z

The aspect build :[tab][tab] case looks like it does the wrong thing. I think the solution is to detect what package the pwd corresponds to and pass that to listBazelRules(cmd.Context(), bzlPackage)

gregmagolan · 2023-02-27T05:37:21Z

Overall this is great. 🙏 for the PR and thanks for your patience.

The only issues I found were the zsh performance (which we can land without fixing), the missing test, coverage and run commands for target completion (pushed the fix for that) & the handling of :[tab][tab] (which I think should get fixed before landing).

Performance is fine with bash so we don't need any optimizations there.

Detecting the package corresponding to the cwd (to handle :[tab][tab] correctly) suppose will need scanning up for the next BUILD (or BUILD.bazel) file up the path and then the WORKSPACE (or WORKSPACE.bazel).

oncilla · 2023-03-27T12:12:59Z

Life got busy. I will try to give this another look this weekend.
But I can't promise anything.

gregmagolan · 2023-05-04T05:34:48Z

Rebased, resolved conflicts & force pushed

alexeagle · 2023-08-31T22:07:56Z

@gregmagolan @oncilla any updates on this one?

This change adds auto completion for bazel flags on all the bazel sub-commands. ``` aspect build --sand<tab><tab> --sandbox_add_mount_pair --sandbox_explicit_pseudoterminal --sandbox_base --sandbox_fake_hostname --sandbox_block_path --sandbox_fake_username --sandbox_debug --sandbox_tmpfs_path --sandbox_default_allow_network --sandbox_writable_path ``` Flag completion for aspect specific flags is not affected: ``` aspect build --aspect:<tab><tab> config (User-specified Aspect CLI config file. /dev/null indic…) interactive (Interactive mode (e.g. prompts for user input) ```

The logic is similar to the bazel logic: https://github.com/bazelbuild/bazel/blob/d1bbf4b7698e019c11b7d1c483eb0b4959954060/scripts/bazel-complete-template.bash#L272-L278 However, not everything is behaving exactly the same, e.g., a custom `BAZEL_COMPLETION_PACKAGE_PATH` is not supported yet. However, in most cases the solution will just work.

oncilla · 2023-11-01T18:21:03Z

@alexeagle @gregmagolan

I finally found some time to work on this. Let me know what you think

oncilla · 2024-02-10T18:02:37Z

@alexeagle @gregmagolan any thing I can do to move this forward?

gregmagolan · 2024-02-12T18:29:07Z

@alexeagle @gregmagolan any thing I can do to move this forward?

Thanks for the reminding @oncilla. I finally have a breather from Aspect Workflows trials & sales this week since we're at a company offsite and told all of our customers we'll be busy this week. Just pulled down your PR. I'll sync it internally and get it landed there this week.

gregmagolan · 2024-02-16T01:59:32Z

I've synced this PR internally and improved the completion logic there. There were some bugs and I ended up simplifying the logic and handling more cases such as labels with @// and @@// user workspace prefixes. It will be landed internally and then copybara'd out to here shortly.

Thanks for the inspiration @oncilla

oncilla · 2024-02-16T09:57:23Z

@gregmagolan awesome! Can't wait to drop my hacky bazel completion script.

Inspired by #373 with a re-write + simplication + improvement of the completion logic from the original. To enable bash completion: ``` $ brew install bash-completion $ aspect completion bash > $(brew --prefix)/etc/bash_completion.d/aspect # add to your .bash_profile: if [[ -r "$HOMEBREW_PREFIX/etc/profile.d/bash_completion.sh" ]]; then . "$HOMEBREW_PREFIX/etc/profile.d/bash_completion.sh" echo "bash completion loaded" fi ``` See https://tecadmin.net/enable-bash-completion-on-macos/ for more info. Demos: ``` $ aspect <tab><tab> analyze-profile (Analyze build profile data) info (Display runtime info about the bazel server) aquery (Query the action graph) init (Create a new Bazel workspace) build (Build the specified targets) license (Prints the license of this software.) canonicalize-flags (Present a list of bazel options in a canonical form) lint (Run configured linters over the dependency graph.) clean (Remove the output tree) outputs (Print paths to declared output files) completion (Generate the autocompletion script for the specified shell) print (Print syntax elements from BUILD files) config (Displays details of configurations.) query (Query the dependency graph, ignoring configuration flags) configure (Auto-configure Bazel by updating BUILD files) run (Build a single target and run it with the given arguments) coverage (Same as 'test', but also generates a code coverage report.) shutdown (Stop the bazel server) cquery (Query the dependency graph, honoring configuration flags) support (Interactive, human-escalated support for Bazel problems) docs (Open documentation in the browser) test (Build the specified targets and run all test targets among them) fetch (Fetch external repositories that are prerequisites to the targets) version (Print the versions of Aspect CLI and Bazel) help ``` ``` $ aspect test <tab><tab> :test :test2 foo foo/bar foo/fum/bar ``` ``` $ aspect test foo<tab><tab> foo foo/bar foo/fum/bar foo:foo foo:fum/test foo:test foo:test2 ``` ``` $ aspect test foo/<tab><tab> foo/bar foo/fum/bar ``` ``` $ aspect test //foo<tab><tab> //foo //foo/bar //foo/fum/bar //foo:foo //foo:fum/test //foo:test //foo:test2 ``` ``` $ aspect test //foo/<tab><tab> //foo/bar //foo/fum/bar ``` ``` $ aspect test @//foo<tab><tab> @//foo @//foo/bar @//foo/fum/bar @//foo:foo @//foo:fum/test @//foo:test @//foo:test2 ``` ``` $ aspect test @//foo/<tab><tab> @//foo/bar @//foo/fum/bar ``` ``` $ aspect test @@//foo<tab><tab> @@//foo @@//foo/bar @@//foo/fum/bar @@//foo:foo @@//foo:fum/test @@//foo:test @@//foo:test2 ``` ``` $ aspect test @@//foo/<tab><tab> @@//foo/bar @@//foo/fum/bar ``` ``` $ cd foo $ aspect test <tab><tab> :foo :fum/test :test :test2 bar fum/bar ``` GitOrigin-RevId: 4044b8d8bb34dfad160cb1d4728bf13d10cb3b9c

gregmagolan · 2024-02-17T14:42:27Z

@oncilla This is now landed, synced back out to this repo and included in the 5.9.7 release. Please give it a try. There are still a few oddities with the suggested completion labels (or lack of labels) that buildozer returns for some BUILD files but generally the behaviour is very good from my testing.

oncilla · 2024-02-17T17:55:16Z

To be fair, the regular bazel completion is also not great. E.g., macros are not expanded for me etc.

I will give it a try on Monday

alexeagle · 2024-02-17T18:29:47Z

It's expected that macros aren't expanded, because Bazel loading phase is too slow to run in tab complete. It just sees the syntax of the BUILD files.

oncilla force-pushed the completion-flag branch from a7e6604 to 3723cbd Compare January 15, 2023 11:36

oncilla changed the title ~~feat: add flag completion for bazel commands~~ feat: add flag and label completion for bazel commands Jan 15, 2023

gregmagolan mentioned this pull request Jan 24, 2023

[Bug]: aspect completion zsh does not contain flags from underlying Bazel #355

Open

1 task

gregmagolan self-requested a review February 5, 2023 05:22

gregmagolan added the enhancement New feature or request label Feb 5, 2023

gregmagolan self-assigned this Feb 5, 2023

gregmagolan added the in review label Feb 18, 2023

gregmagolan force-pushed the completion-flag branch from b951e36 to 967eec4 Compare February 27, 2023 03:41

gregmagolan reviewed Feb 27, 2023

View reviewed changes

gregmagolan force-pushed the completion-flag branch from 2d480a0 to 3e4d674 Compare May 4, 2023 05:34

oncilla added 3 commits October 31, 2023 21:00

Add completion support for startup flags

48a94df

oncilla force-pushed the completion-flag branch from 3e4d674 to 5f3c19c Compare October 31, 2023 20:01

oncilla added 2 commits October 31, 2023 21:15

fix-rebase

7289b1d

feedback

f07c772

gregmagolan closed this Feb 16, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add flag and label completion for bazel commands #373

feat: add flag and label completion for bazel commands #373

oncilla commented Jan 15, 2023 •

edited

Loading

CLAassistant commented Jan 15, 2023 •

edited

Loading

gregmagolan commented Jan 24, 2023 •

edited

Loading

oncilla commented Jan 25, 2023 •

edited

Loading

gregmagolan commented Jan 25, 2023

oncilla commented Feb 7, 2023

gregmagolan Feb 27, 2023

gregmagolan Feb 27, 2023

oncilla Oct 31, 2023

gregmagolan Feb 27, 2023 •

edited

Loading

oncilla Oct 31, 2023

gregmagolan commented Feb 27, 2023

gregmagolan commented Feb 27, 2023

gregmagolan commented Feb 27, 2023 •

edited

Loading

oncilla commented Mar 27, 2023

gregmagolan commented May 4, 2023 •

edited

Loading

alexeagle commented Aug 31, 2023

oncilla commented Nov 1, 2023

oncilla commented Feb 10, 2024

gregmagolan commented Feb 12, 2024

gregmagolan commented Feb 16, 2024

oncilla commented Feb 16, 2024

gregmagolan commented Feb 17, 2024 •

edited

Loading

oncilla commented Feb 17, 2024

alexeagle commented Feb 17, 2024

feat: add flag and label completion for bazel commands #373

feat: add flag and label completion for bazel commands #373

Conversation

oncilla commented Jan 15, 2023 • edited Loading

CLAassistant commented Jan 15, 2023 • edited Loading

gregmagolan commented Jan 24, 2023 • edited Loading

oncilla commented Jan 25, 2023 • edited Loading

gregmagolan commented Jan 25, 2023

oncilla commented Feb 7, 2023

gregmagolan Feb 27, 2023

Choose a reason for hiding this comment

gregmagolan Feb 27, 2023

Choose a reason for hiding this comment

oncilla Oct 31, 2023

Choose a reason for hiding this comment

gregmagolan Feb 27, 2023 • edited Loading

Choose a reason for hiding this comment

oncilla Oct 31, 2023

Choose a reason for hiding this comment

gregmagolan commented Feb 27, 2023

gregmagolan commented Feb 27, 2023

gregmagolan commented Feb 27, 2023 • edited Loading

oncilla commented Mar 27, 2023

gregmagolan commented May 4, 2023 • edited Loading

alexeagle commented Aug 31, 2023

oncilla commented Nov 1, 2023

oncilla commented Feb 10, 2024

gregmagolan commented Feb 12, 2024

gregmagolan commented Feb 16, 2024

oncilla commented Feb 16, 2024

gregmagolan commented Feb 17, 2024 • edited Loading

oncilla commented Feb 17, 2024

alexeagle commented Feb 17, 2024

oncilla commented Jan 15, 2023 •

edited

Loading

CLAassistant commented Jan 15, 2023 •

edited

Loading

gregmagolan commented Jan 24, 2023 •

edited

Loading

oncilla commented Jan 25, 2023 •

edited

Loading

gregmagolan Feb 27, 2023 •

edited

Loading

gregmagolan commented Feb 27, 2023 •

edited

Loading

gregmagolan commented May 4, 2023 •

edited

Loading

gregmagolan commented Feb 17, 2024 •

edited

Loading